Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NH-20004: NH Java agent: upgrade Otel dependency to the latest version #74

Merged
merged 13 commits into from
Feb 10, 2023

Conversation

chukwubuikem-umeugwa_swi
Copy link
Mannequin

This PR addresses this and this ticket.
To accomplish this, the project was refactored to follow similar configuration as is found here and ensuring that any dependency on OTEL is added using compileOnly configuration. This ensures that bootstrap class loader and the agent class loader don't end up loading the same exact class.

The buildSrc directory was removed because the new build configuration uses the Gradle instrumentation plugin making it unnecessary. Also added maven local repository installation to facilitate local testing

jiwen624 and others added 10 commits August 25, 2022 16:22
# Conflicts:
#	custom/src/main/java/com/appoptics/opentelemetry/extensions/AppOpticsTracerProviderConfigurer.java
# Conflicts:
#	instrumentation/servlet/servlet-3.0/javaagent/src/main/java/com/appoptics/opentelemetry/instrumentation/servlet/v3_0/Servlet3Advice.java
#	instrumentation/servlet/servlet-5.0/javaagent/src/main/java/com/appoptics/opentelemetry/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004)
- remove reference to SL4J logger due to issue with class loading
- exclude resource from shadow relocation
- exclude `io/opentelemetry/javaagent/shaded/instrumentation/api/**` from angent loader path as this is available in the bootstrap path.
@chukwubuikem-umeugwa_swi chukwubuikem-umeugwa_swi mannequin changed the title Cc/nh 20004 nuke NH-20004: NH Java agent: upgrade Otel dependency to the latest version Feb 8, 2023
- [JIRA](https://swicloud.atlassian.net/browse/NH-20004)
- It turns out that we have to use the builder to create a working tracer. Though using `GlobalOpenTelemetry` is still viable, it is actually discouraged and requires nasty workaround. With the new release the tooling that made using `GlobalOpenTelemetry` work were removed(`ApplicationTracer`).
Copy link
Mannequin

@tingfung-leung_swi tingfung-leung_swi mannequin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

- [JIRA](https://swicloud.atlassian.net/browse/NH-20004)
- remove shadow tasks from agent libs build.gradle
- exclude bootstrap classes from agent class loader path
@chukwubuikem-umeugwa_swi chukwubuikem-umeugwa_swi mannequin dismissed a stale review via cea2129 February 9, 2023 21:04
Lin-Lin_swi
Lin-Lin_swi mannequin approved these changes Feb 10, 2023
Copy link
Mannequin

@Lin-Lin_swi Lin-Lin_swi mannequin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thank you for sorting through the rat's nest of classloading and upstream changes and putting our distro, finally, in a much better place @cleverchuk! (guess what, OTel 1.23.0 is on the way ;))

I didn't do so much a code review as a set of manual tests, checking some basic functionality like:

  • trace spans, metrics, profiling spans - OK
  • recent exporter and __Init message changes - OK
  • your fix to response header - OK

One representative trace in staging being https://my.na-01.st-ssp.solarwinds.com/136477216875174912/traces/2EDFF3AEBCCF3EB2245CCA697712C099/4FE52F36BDD41034/details/breakdown?duration=3600, which has errors and code profiling.

@chukwubuikem-umeugwa_swi chukwubuikem-umeugwa_swi mannequin merged commit 3fb5d7a into main Feb 10, 2023
@chukwubuikem-umeugwa_swi chukwubuikem-umeugwa_swi mannequin deleted the cc/NH-20004-nuke branch April 9, 2024 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants